Repository navigation
test(macapp): a bell row selects the tab that raised it, pinned to #598's focus guard - #609
Merged
Merged
Conversation
…'s focus guard (#607) Clicking a bell row for a notification raised by another tab of the workroom in front left the front tab selected. The routing was always right (the record carries its tab id; WindowRegistry.open -> openTerminal -> applyLocation focuses it), but the closing popover handed first responder back to the old pane, whose .focused took focus back. #598's isFocusBeingShown guard fixed that mechanism; #607 was seen on a branch cut before #598. No behaviour change: NotificationRoutingTests and a WorkroomWorkflowUITests case pin it (both red with the guard disabled, green on master), plus a closed-tab fallback test, and the guard's comment names the notification path. Fixes #607 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QxGw2gUmeT3FCNK7RnCdky
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Opened by Claude Code.
Summary
#607: clicking a bell row for a notification raised by another tab of the workroom in front left the front tab selected. #598 (f20ab36) already fixed this on master. #607 was seen on the #597 branch, whose base (e2d2a81) predates #598.
The bell, toast, menu-bar popover, ⇧⌘N and needs-you rows all take this route and pass the same guard. Only the same workroom was affected: across workrooms, the stale
.focusedbelongs to another target.This PR changes no behaviour. It pins the fix with tests and names the notification path in the guard's comment.
Evidence
NotificationRoutingTests.testOpeningANotificationSelectsTheBackgroundTabThatRaisedIt: with the guard disabled,Tests: Failed - 7 passed, 1 failed (of 8); on master, 8/8.WorkroomWorkflowUITests.testABellRowFromAnotherTabOfTheSameWorkroomSelectsThatTab, which follows the issue's steps in a Tart guest: it fails with the guard disabled and passes on master. The whole class passes 4/4.testANotificationWhoseTabClosedOpensItsWorkroomOnTheTabInFrontguards against regressions.make app-testgaveTests: Passed - 3398 passed, 0 failed, 106 skipped (of 3504).make app-format,app-buildandapp-lintare clean.Merge Danger
Door: two-way. Blast Radius: none; tests and one comment.
Fixes #607
🤖 Generated with Claude Code
https://claude.ai/code/session_01QxGw2gUmeT3FCNK7RnCdky